-
Notifications
You must be signed in to change notification settings - Fork 9
Update Hub setup wizard to Hub 1.4.0 with Keycloak 26.x #121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughThe changes update the authentication and configuration mechanisms for the system by replacing the previous "syncer" user and password approach with a new "system" service account model that uses a client secret. In the configuration, the Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
assets/js/hubsetup.js (1)
36-36
: Consider using a cryptographically secure random generator.
The newsystemClientSecret
relies onHubSetup.uuid()
, which appears to useMath.random()
. For added security, consider introducing a more robust random secret generator.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
assets/js/hubsetup.js
(16 hunks)
🔇 Additional comments (13)
assets/js/hubsetup.js (13)
115-115
: Versioning update acknowledged.
Updating the header to indicate "script version 7" is consistent with the new configuration.
245-246
: Verify scope of 'realm-admin' role for the system account.
Assigning the system user as 'realm-admin' grants it broad privileges. Confirm this level of access is intended or consider limiting privileges to reduce risk.Also applies to: 248-249
315-324
: Service account client configuration looks correct.
Defining a dedicated system client with a client secret and disabling standard flow is aligned with Keycloak’s recommended usage for machine-to-machine integration.
431-431
: Keycloak image version updated.
Switching the image toghcr.io/cryptomator/keycloak:26.1.5
aligns with the stated upgrade objectives.
441-441
: Confirm Keycloak container listens on port 9000.
The liveness check now targetslocalhost:9000
; ensure the container indeed binds to that port for health checks.
454-457
: Check Keycloak hostname and proxy headers.
SettingKC_HOSTNAME
to null in dev mode andKC_PROXY_HEADERS
to "xforwarded" in production is a valid approach. Confirm that a null environment variable is well-handled by Keycloak in dev mode.
478-478
: Mixed-port health check.
The health check runs on port 9000 for/q/health/live
and port 8080 for/api/config
. Verify that both endpoints are reachable as intended.
488-489
: Use of system client credentials is consistent.
Replacing syncer credentials with system credentials matches the new model.
498-498
: Enabling proxy address forwarding.
SettingQUARKUS_HTTP_PROXY_PROXY_ADDRESS_FORWARDING
to true is appropriate if the environment is behind a reverse proxy.
656-657
: Adjust wait-for-keycloak checks.
The initContainer now waits on port 9000 for Keycloak's /health/live. Confirm Keycloak’s internal port mapping aligns with 9000.
682-683
: New system client environment variables.
Using a secret reference forHUB_KEYCLOAK_SYSTEM_CLIENT_SECRET
and a fixed ID forHUB_KEYCLOAK_SYSTEM_CLIENT_ID
is good practice.
802-802
: Keycloak image reference updated.
Usingghcr.io/cryptomator/keycloak:26.1.5
aligns with the intended version upgrade.
810-810
: Ensure liveness/readiness endpoints match port 9000.
Both probes now point to port 9000, which differs from the container’s default port 8080. Ensure the Keycloak container is configured accordingly.Also applies to: 815-815
Still needs proper testing during release.